Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Drozdov_Nikita#38

Open
pepya4ka wants to merge 58 commits intomasterfrom
feature/DrozdovNikita
Open

Drozdov_Nikita#38
pepya4ka wants to merge 58 commits intomasterfrom
feature/DrozdovNikita

Conversation

@pepya4ka
Copy link
Collaborator

No description provided.

@NikolaevArtem NikolaevArtem added the bug Code fix needed label Jul 11, 2021
@NikolaevArtem NikolaevArtem changed the title DrozdovNikita_homework Drozdov_Nikita Jul 11, 2021
@pepya4ka pepya4ka added readyForReview Sign for Artem to take a look bug Code fix needed and removed bug Code fix needed labels Jul 12, 2021
@NikolaevArtem NikolaevArtem added HW_1 Good to go and removed bug Code fix needed readyForReview Sign for Artem to take a look labels Jul 13, 2021
@pepya4ka pepya4ka added readyForReview Sign for Artem to take a look and removed readyForReview Sign for Artem to take a look labels Jul 15, 2021
@pepya4ka pepya4ka added the readyForReview Sign for Artem to take a look label Jul 15, 2021
@AlexandrKirshin AlexandrKirshin added readyForReview Sign for Artem to take a look and removed readyForReview Sign for Artem to take a look labels Jul 19, 2021
@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Jul 20, 2021
@pepya4ka pepya4ka added the readyForReview Sign for Artem to take a look label Jul 21, 2021
@NikolaevArtem NikolaevArtem removed readyForReview Sign for Artem to take a look style Style fix needed readyForStudentReview labels Sep 20, 2021
import java.util.Objects;
import java.util.stream.Collectors;

public class XmlConverter {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea and implementation!

public class Main {

public static void main(String[] args) {
new CustomFileReader().run1();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2nd case somehow broken:
image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice usage of abstractions!

public class Main {

public static void main(String[] args) {
new CustomRegexMatcher().run();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, real-life regex example with optional borders. Well-done!

public class Main {

public static void main(String[] args) {
new PowerOfNumber().run();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Usage of BigInteger

}

private BigInteger pow(BigInteger a, BigInteger b) {
if (b.compareTo(BigInteger.valueOf(0)) == 0) return BigInteger.valueOf(1);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (b.compareTo(BigInteger.valueOf(0)) == 0) return BigInteger.valueOf(1);
if (b.compareTo(BigInteger.valueOf(0)) == 0) {
return BigInteger.valueOf(1);
}

Opt

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, assigning new data to the same variables is usually a code smell

Comment on lines 72 to 73
private void runMapProblemsCollision() {
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No examples for collision?

Comment on lines 20 to 24
@Override
public int hashCode() {
return new Random().nextInt(10);
// return new Random().nextInt();
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with mutable keys is that you can't find the value if you change the key. In your case, it's just hashcode doesn't follow contracts. Even with properly-written hashcode mutable objects as keys can be lost in case of changing (and rehashing as the result)


import static homework_7.model.Sex.*;

public class Main {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D nice, good examples!


import static course_project.services.GameService.*;

public class Game extends BaseClazz {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quite difficult to put ships manually, especially because the field does not refresh

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it, you refresh after the type of ships was placed. Smart move

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some bug in field rendering
image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also time between turns is quite long, could you please reduce sleep time?

@NikolaevArtem NikolaevArtem added the bug Code fix needed label Sep 20, 2021
Copy link
Owner

@NikolaevArtem NikolaevArtem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite good! Found some bugs in HW_6 and course project, please fix. In general looks nice, easy to understand, good usage of abstractions and Java Core, as well as Java libraries

@pepya4ka pepya4ka added the readyForReview Sign for Artem to take a look label Sep 21, 2021
@NikolaevArtem NikolaevArtem added Course Project HW_6 and removed bug Code fix needed readyForReview Sign for Artem to take a look labels Sep 23, 2021
@NikolaevArtem
Copy link
Owner

HW and Cource Project approved! Although you could do better on course project, it is working now and I didn't reproduce the last bug and didn't see any new. If you have time, please refactor and ping me. If not, it's fine as is

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants